Produce independent mvn packages for java bindings #98
Conversation
…ter having moved the runtime compliance tests to a different package.
| set -euo pipefail | ||
|
|
||
| mkdir -p conf | ||
| /usr/bin/env node ./user.xml.t.js > conf/user.xml |
There was a problem hiding this comment.
What's the difference between just running node and /usr/bin/env node?
There was a problem hiding this comment.
That's a good Q. Honestly I just copied the style that was in the pom.xml.t.js file without thinking much about it.
There was a problem hiding this comment.
I think there is no technical difference /usr/bin/env is useful for shebangs to actually be able to run a program that's discoverable in PATH (because shebangs need a program as the first arg).
| .version(VERSION) | ||
| .argv; | ||
|
|
||
| const target = new (await Target.all)[argv.target]({ |
There was a problem hiding this comment.
Add a comment that says argv.target is ensured to be valid by yargs "choices"
| const force = argv.force; | ||
| const packageDir = argv._[0]; | ||
| await target.generateCode(codeDir); | ||
| if (!argv.codeOnly) { |
There was a problem hiding this comment.
I would bail-out early in the special-case: if (argv.codeOnly) { return; }
There was a problem hiding this comment.
✅ I initially wrote this assuming there would be something after build that could be relevant also if codeOnly, but that feels highly improbable now.
| 'modelVersion': '4.0.0', | ||
| 'name': `Java bindings for ${assm.name}`, | ||
|
|
||
| ...assm.targets.java.maven, |
There was a problem hiding this comment.
One of the nice things of ES6 :)
|
|
||
| this.code.openFile('pom.xml'); | ||
| this.code.line( | ||
| xmlbuilder.create({ |
| } | ||
|
|
||
| public async build(srcDir: string, outDir: string) { | ||
| await fs.copy(srcDir, outDir, { overwrite: true, recursive: true }); |
There was a problem hiding this comment.
For overwrite, yes, but the documentation makes no mention of recursive.
There was a problem hiding this comment.
I mean, isn't this the default for Target, so you don't really need to override build here...
There was a problem hiding this comment.
It suppresses the message saying there's no "build phase" for the "PackOnly" target.
There was a problem hiding this comment.
Yeah... I guess I could remove the warning from the default... What do you think about this?
There was a problem hiding this comment.
Perhaps make build abstract and supply a protected utility for copySource so implementors will explicitly indicate this is their intention
| @@ -0,0 +1,120 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
| <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd"> | |||
There was a problem hiding this comment.
Wasn't there a comment about this is generated code somewhere?
There was a problem hiding this comment.
That is part of a tree that, axiomatically, contains only generated code... If I label that one file, I should also label every other file...
| </properties> | ||
| <dependencies> | ||
| <dependency> | ||
| <groupId>com.amazonaws</groupId> |
There was a problem hiding this comment.
Please change the groupId for jsii-java-runtime to com.amazonaws.jsii (requested by the open source group)
There was a problem hiding this comment.
Okey - that's super duper easy now :P
| import path = require('path'); | ||
|
|
||
| export const maven = { | ||
| groupId: 'com.amazonaws', |
There was a problem hiding this comment.
Change to com.amazonaws.jsii and resolve #102 (needed for dev preview)
| @@ -0,0 +1,15 @@ | |||
| #!/bin/bash | |||
| set -euo pipefail | |||
There was a problem hiding this comment.
Add a little comment what this script is about
Fist step towards #73:
jsii-pacmakproduce a full maven project for each module being builtjsii-pacmakbuild the resulting java project (unless--only-sourceis passed)jsii-pacmakpublish the artifacts (or source if--only-sourceis passed) to the--outdirjsii-java-runtimeand related compliance tests